Skip to content

docs(svelte): remove using component warning from rendered results #299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 14, 2019
Merged

Conversation

mihar-22
Copy link
Contributor

No description provided.

@mihar-22 mihar-22 merged commit 91587ab into testing-library:master Oct 14, 2019
@alexkrolick
Copy link
Collaborator

@kentcdodds ?

| Result | Description |
| ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `container` | The HTML element the component is mounted into. |
| `component` | The newly created Svelte component. This is mostly useful for testing exported functions or cases where manipulating the DOM doesn't fit. Outside of said cases avoid using the component directly to build tests, instead of interacting with the rendered Svelte component, work with the DOM . |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain such a scenario. This seems to go against the core philosophy of Testing Library and I want to make sure anyone reading this doesn't get the wrong idea that they should be testing implementation details.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update was in reference to this issue.

So before I had touched the svelte-testing-library or the docs the component was already being returned and the docs had no warning at all. My first update couldn't remove the component (backwards compatibility), and so I left a warning comment in the source code, and I wrote a warning (I recently removed) in the new docs.

However, in Svelte you can export functions from a component that can be used on the rendered instance.

Example:

<script>
// Component.svelte

export function myFunction() {
  // Performs some operation...
}
</script>
// Test.js

const { component } = render(Component)

// This is the only way to call this method.
component.myFunction()
// You might export a "static" function and use it like this.
component.prototype.myFunction()

You can think of these functions as equivalent to a method OR static method in a React component, with the main difference being that this method is exposed as part of the client API. In React you don't compile components that can be used as vanilla JS components.

In the end I thought there may be edge cases where people need to access these exported functions so we should just return out of convenience for those situations. Otherwise people would need to initialize their components without the render function and their tests would start getting messy. It'd be best to just rely on the testing library and not duplicate code.

I thought my explanation in the docs was okay because I described the use case and I still mentioned, "Outside of said cases avoid using the component directly to build tests, instead of interacting with the rendered Svelte component, work with the DOM".

Love to hear any suggestions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment about some of my thoughts on the specific situation here: testing-library/svelte-testing-library#57 (comment)

If I understand you correctly, you're saying that it's common in svelte for a parent component to access and call directly the methods of a child component? If that's the case, then it makes sense to expose the component in this way (the developer-user is one of the users we need to write tests for after all). But if that's not correct, then we need to make sure that people's tests resemble the way their software is used and if the only place they're accessing component methods is in their tests, then that's problematic.

Copy link
Contributor Author

@mihar-22 mihar-22 Oct 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I completely agree with everything you've said, including your comments on the issue. Unit tests like the third test in the issue are brittle and don't add much value, the fourth test I also mentioned can be rewritten to be interacted with through the DOM, just like a user would.

I may have to rewrite the docs to better explain when it is acceptable. Due to Svelte being a compiler and not a framework, many people not only create user facing applications/components but they also compile vanilla js components that have developer facing API's.

Example

Let's say I was creating a video player component with svelte to be distributed and used in other projects, and I had a play function I wanted to be made available on the compiled JS component, so devs of my player can control the video player programmatically and call play when they want to.

<script>
// VideoPlayer.svelte

export function play() {
}
</script>

What developers would then be able to do with the compiled code is

const videoPlayer = new VideoPlayer()

// Call this method to play the video
videoPlayer.play()

In a testing environment if you wanted to test whether the play button did what you expected without using the component directly, you'd need to create a fixture (wrapper component, add a button that when pressed calls videoPlayer.play()) and see if the video is playing. All of that would be useless boilerplate and not actually represent how the component is being used.

So this is the use case I was alluding to. I only see the component useful for testing the developer facing API's of svelte components. I thought it'd be a convenient export for those situations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I agree with your conclusions. It just needs to be really clear when it's useful and when it's not. Maybe linking to https://kcd.im/test-user would help.

superT999 added a commit to superT999/testing-library-docs that referenced this pull request Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants